Fix presets in themes that use the default color & gradient palettes #22526
Conversation
oandregal
requested review from
ajitbohra,
jorgefilipecosta,
Soean and
talldan
as code owners
oandregal
added
the
[Feature] Custom Editor Styles
oandregal
changed the title
oandregal
changed the title
oandregal
changed the title
|
Size Change: +399 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
| @@ -37,6 +37,7 @@ | |||
| // to assure colors take effect over another base class color, mainly to let | |||
| // the colors override the added specificity by link states such as :hover. | |||
|
|
|||
| :root .editor-styles-wrapper, | |||
| :root { | |||
There was a problem hiding this comment.
Why not just remove root?
There was a problem hiding this comment.
The issue here is that theme and core have the same specificity (020), hence the last declaration wins: changing classes won't solve the issue.
By doing this the specificity in the front will still be 020 (root pseudoclass + preset class) while in the editor is 030 (root + editor wrapper class + preset class). My understanding from reading past convos was that we used :root to cover focus and hover statuses for colors, so we still need something like that. I could perhaps look into change :root by those more specific classes, however, root seems to be working fine for that purpose?
There was a problem hiding this comment.
I thought .editor-styles-wrapper .color was enough since the other conflicting rules are more .editor-styles-wrapper button but I think your approach is safer but why not apply it to font-sizes. I guess we could have the same issue there too.
There was a problem hiding this comment.
I just double-checked, and TwentyTwenty uses the button class, not the element (.editor-styles-wrapper .wp-block-button__link). Probably other themes do this differently, though.
Adding a new selector with the editor wrapper is the same thing I did for font-sizes a few PRs back, so doing this for colors as well makes everything coherent.
There was a problem hiding this comment.
Adding a new selector with the editor wrapper is the same thing I did for font-sizes a few PRs back, so doing this for colors as well makes everything coherent.
Should we add :root there too?
There was a problem hiding this comment.
🤔 what would be the advantage? I believe for colors it was only introduced to cover hover/focus statuses, so perhaps it makes more sense to change :root to :hover & :focus (although not sure if there are browsers issues with them).
There was a problem hiding this comment.
For the same reasons maybe? Don't you think we need to protect these for fonts too?
There was a problem hiding this comment.
My worry is that I haven't seen reports of that being an issue. I guess color is something people change for those hover/focus statutes (so they enqueue styles) but fonts don't, hence there's no conflict. However, I'm always happy to explore and see what happens. I can prepare a separate PR to gather feedback and address that.
There was a problem hiding this comment.
Created a PR to discuss this separately at #22671
Fixes #22482